Skip to content

Conversation

@vpillinger
Copy link

@vpillinger vpillinger commented Jan 3, 2017

Included dialog addons for pretty search dialogs. #58

index.html Outdated

<!-- Enable CodeMirror's search addons -->
<script src="node_modules/codemirror/addon/search/search.js" type="text/javascript"></script>
<script src="node_modules/codemirror/addon/search/jumpToLine.js" type="text/javascript"></script>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks like this should be jump-to-line.js, I'm getting an error otherwise

Copy link
Author

@vpillinger vpillinger Jan 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is strange. You are right about that import, but it worked for me initially and fixing it didn't seem to change anything. So I'm not sure what the deal with that is on my local machine.

@wellsjo
Copy link
Owner

wellsjo commented Jan 4, 2017

Can we improve the UX a bit here? Searching from the command line doesn't work, for example. I don't think simply enabling CodeMirror's search is enough.

@vpillinger
Copy link
Author

I agree with you that the default code-mirror UX is pretty ugly, requiring Cmd+G to browse through results. So this PR doesn't finish the search feature, but it certainly moves it in the right direction while someone can work on improving the dialog boxes.
Also, don't know what you mean about searching from the command line. The CLI is right now only a shortcut to open a file. Why wouldn't a user just open the file then press Ctrl+F or Cmd+F to search after opening the file? I think there is a use-case here that I am not seeing.

@wellsjo
Copy link
Owner

wellsjo commented Jan 21, 2017

Hey sorry this took so long for me to get to, I was away for a while. Could you rebase and then I will check it out/merge?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants